🐛(i18n/backend) Fix/email in receiving user language #401

rvveber · 2024-11-04T14:02:08Z

Implements the following:

E-mails sent for granting access are sent in the users language if it exists - else they will be sent in system default language
On Language change in the frontend, the language attribute of the user is updated via API.
Fixes to language related Tests
Added helper for language related Tests

Closes #323

rvveber · 2024-11-21T11:40:59Z

@sampaccoud
Would it make sense to refactor the backend user.language choices to ISO639-1?
eg. fr-fr -> fr

AntoLC

Looks good !
A few comments.

@sampaccoud Should we add a migration to make all our current users french by default, because they will all switch to english after that, wdyt ?

src/frontend/apps/impress/src/features/language/api/useChangeUserLanguage.tsx

src/frontend/apps/e2e/__tests__/app-impress/common.ts

src/frontend/apps/impress/src/core/AppProvider.tsx

src/frontend/apps/impress/src/features/language/api/useChangeUserLanguage.tsx

src/frontend/apps/impress/src/features/language/LanguagePicker.tsx

src/backend/core/api/viewsets.py

src/frontend/apps/impress/src/features/language/LanguagePicker.tsx

src/frontend/apps/impress/src/i18n/conf.ts

src/backend/core/tests/documents/test_api_document_invitations.py

AntoLC

🎉 Good job, works perfect ^^

I think Sam wants to have a look at it before merging.

sampaccoud · 2024-12-01T11:17:34Z

I was expecting the language to be set on the user on first logging here:
https://github.com/numerique-gouv/impress/blob/main/src/backend/core/authentication/backends.py#L91

Otherwise how and when is this field set on the user? 🤔

src/backend/core/tests/documents/test_api_document_invitations.py

AntoLC

A few comments but it seems good to me, all seems working fine.

We try to not mix backend and frontend in the same commit, to be easier for reviewers, some of us are specialized in backend, other in front.

AntoLC · 2025-02-27T14:08:13Z

src/frontend/apps/e2e/__tests__/app-impress/language.spec.ts

+  const [response] = await Promise.all([
+    page.waitForResponse(
+      (resp) =>
+        resp.url().includes('/user') && resp.request().method() === 'PATCH',
+    ),
+    page.getByLabel(lang.label).click(),
+  ]);


Suggested change

const [response] = await Promise.all([

page.waitForResponse(

(resp) =>

resp.url().includes('/user') && resp.request().method() === 'PATCH',

),

page.getByLabel(lang.label).click(),

]);

const response = page.waitForResponse(

(resp) =>

resp.url().includes('/user') && resp.request().method() === 'PATCH',

);

await page.getByLabel(lang.label).click();

AntoLC · 2025-02-27T14:16:22Z

src/frontend/apps/impress/src/features/left-panel/components/LefPanelTargetFilters.tsx

+  const defaultQueries = [
+    {
+      icon: 'apps',
+      label: t('All docs'),
+      targetQuery: DocDefaultFilter.ALL_DOCS,
+    },
+    {
+      icon: 'lock',
+      label: t('My docs'),
+      targetQuery: DocDefaultFilter.MY_DOCS,
+    },
+    {
+      icon: 'group',
+      label: t('Shared with me'),
+      targetQuery: DocDefaultFilter.SHARED_WITH_ME,
+    },
+  ];



The use of useMemo is dicutable, but is this change necessary ? It seems to work correctly on my side.

Function t was not changing, when language was updated automatically to whatever is defined on the user.
Not sure why it happens, seems to be an issue of async nature.

Here is how you can reproduce it:

Switch back to the variant with useMemo

Log in to django backend.

Go to frontend (new tab)

Change language to french

Go to django backend and change superusers language to any other eg. deutsch

Switch back to frontend tab and reload

Left panel still has old translation

AntoLC · 2025-02-27T14:18:04Z

src/frontend/apps/e2e/__tests__/app-impress/language.spec.ts

+test.afterEach(async ({ page }) => {
+  // Switch back to English - important for other tests to run as expected
+  await waitForLanguageSwitch(page, TestLanguage.English);
+});


Could we add it to the top ?

AntoLC · 2025-02-27T14:26:19Z

src/frontend/apps/impress/src/core/AppProvider.tsx

@@ -41,7 +41,10 @@ export function AppProvider({ children }: { children: React.ReactNode }) {
    <QueryClientProvider client={queryClient}>
      <CunninghamProvider theme={theme}>
        <ConfigProvider>
-          <Auth>{children}</Auth>
+          <Auth>
+            <LanguageInitializer />


Let's add it in the ConfigProvider, it seems more appropriate.

Are you sure?
I need auth to access the language of the user, logic in ConfigProvider does not need auth (yet)

It will add quite a bit of bloat to the ConfigProvider, better to have everything encapsulated in components and use them where they are needed right?

It can work if i make LanguageInitializer an async hook as you suggested below.
working on it

AntoLC · 2025-02-27T14:32:49Z

src/frontend/apps/impress/src/features/language/LanguageInitializer.tsx

@@ -0,0 +1,66 @@
+import i18n from 'i18next';


Better to get it like that:

import { useTranslation } from 'react-i18next'; ... const { i18n } = useTranslation();

AntoLC · 2025-02-27T14:38:18Z

src/frontend/apps/impress/src/features/language/LanguageInitializer.tsx

+
+import { getMatchingLocales } from './utils/locale';
+
+const LanguageInitializer = () => {


If the component doesn't have any content, better to make it as a hook, wdyt ?

Sounds good

- language for invitation emails => language saved on the invited user - if invited user does not exist yet => language of the sending user - if for some reason no sending user => system default language

- allow the language on the user to be unset - set the default language to be unset - helps us determine that the user has yet to set a language preference

- allow the language attribute on the user to be updated via API - add frontend function to update the user language via API - extend defaults on the test users, to have fixed language in E2E tests - extend types and variables using the types with the new field

- Adds a helper for working with locales - More details in their annotations - Unnecessary, if in the future, the backend uses the same locales as the keys in the translations (ISO 639-1)

- config endpoint languages are used as available options for LanguagePicker - updating the language from it, triggers an update on the user via API

- ensure editor is translated to i18n.resolvedLanguage => en as i18n.language could hold more accurate locale => en-GB etc..

- adds useLanguageSynchronizer hook to update the: 1. frontend-language to the user-preference - if there is one. 2. user-preference to the (browser-detected) frontend-language - otherwise.

- fixes a bug where after language-sync the left panel would remain in the same language as before.

- adds tests and test-utility for solid language switching in tests - fixes where ...getByRole(menuitem... would not return a valid object

virgile-dev · 2025-03-05T12:47:03Z

@sampaccoud could you approve and merge ?

sampaccoud · 2025-03-05T13:17:12Z

@rvveber Amazing fix! Thanks for that Robin 💪 🎉

rvveber force-pushed the fix/email-in-receiving-user-language branch 2 times, most recently from c645b5b to 72b5272 Compare November 4, 2024 14:10

rvveber marked this pull request as draft November 4, 2024 14:27

rvveber force-pushed the fix/email-in-receiving-user-language branch 4 times, most recently from 62ebf55 to 332e770 Compare November 18, 2024 17:20

rvveber force-pushed the fix/email-in-receiving-user-language branch 6 times, most recently from fef3dea to 3b3e90c Compare November 21, 2024 16:46

rvveber requested review from AntoLC and sampaccoud November 21, 2024 16:55

rvveber marked this pull request as ready for review November 21, 2024 16:56

AntoLC reviewed Nov 25, 2024

View reviewed changes

rvveber force-pushed the fix/email-in-receiving-user-language branch 2 times, most recently from b3534d3 to 704550b Compare November 25, 2024 14:50

rvveber requested a review from AntoLC November 25, 2024 14:51

rvveber force-pushed the fix/email-in-receiving-user-language branch from 704550b to d9bad8c Compare November 25, 2024 14:53

AntoLC reviewed Nov 26, 2024

View reviewed changes

src/frontend/apps/impress/src/features/language/LanguagePicker.tsx Outdated Show resolved Hide resolved

src/frontend/apps/impress/src/i18n/conf.ts Outdated Show resolved Hide resolved

src/backend/core/tests/documents/test_api_document_invitations.py Show resolved Hide resolved

rvveber force-pushed the fix/email-in-receiving-user-language branch from d9bad8c to f773350 Compare November 27, 2024 14:30

rvveber requested a review from AntoLC November 27, 2024 14:32

rvveber force-pushed the fix/email-in-receiving-user-language branch 2 times, most recently from 1b5eb7b to 38639c9 Compare November 27, 2024 14:44

AntoLC approved these changes Nov 28, 2024

View reviewed changes

sampaccoud reviewed Dec 1, 2024

View reviewed changes

src/backend/core/tests/documents/test_api_document_invitations.py Outdated Show resolved Hide resolved

rvveber requested a review from AntoLC February 25, 2025 19:45

AntoLC reviewed Feb 27, 2025

View reviewed changes

rvveber force-pushed the fix/email-in-receiving-user-language branch 3 times, most recently from ffe4ad4 to 40d8831 Compare March 4, 2025 09:30

rvveber marked this pull request as draft March 4, 2025 10:26

rvveber force-pushed the fix/email-in-receiving-user-language branch from 40d8831 to a313c91 Compare March 4, 2025 15:13

rvveber added 2 commits March 4, 2025 16:14

🔨(backend) email invitation in invited user's language

524bc76

- language for invitation emails => language saved on the invited user - if invited user does not exist yet => language of the sending user - if for some reason no sending user => system default language

🔨(frontend) email invitation in invited user's language

1a916fb

- language for invitation emails => language saved on the invited user - if invited user does not exist yet => language of the sending user - if for some reason no sending user => system default language

rvveber force-pushed the fix/email-in-receiving-user-language branch from a313c91 to 70704d7 Compare March 4, 2025 15:31

rvveber marked this pull request as ready for review March 4, 2025 15:31

rvveber force-pushed the fix/email-in-receiving-user-language branch from 70704d7 to b0a5b71 Compare March 4, 2025 15:37

rvveber added 10 commits March 4, 2025 16:40

🔨(backend) make the 'language' attribute on the User model nullable

31aa259

- allow the language on the user to be unset - set the default language to be unset - helps us determine that the user has yet to set a language preference

✨(frontend) add language utility for "locale"

233a2bf

- Adds a helper for working with locales - More details in their annotations - Unnecessary, if in the future, the backend uses the same locales as the keys in the translations (ISO 639-1)

✨(backend) the LanguagePicker now uses config as options

6d63687

- config endpoint languages are used as available options for LanguagePicker - updating the language from it, triggers an update on the user via API

✨(frontend) the LanguagePicker now uses config as options

455ef2d

- config endpoint languages are used as available options for LanguagePicker - updating the language from it, triggers an update on the user via API

🐛(frontend) set toolbar-popup to current language

18c9a6b

- ensure editor is translated to i18n.resolvedLanguage => en as i18n.language could hold more accurate locale => en-GB etc..

✨(app) get language from backend; set browser-detected language if null

6844eb3

- adds useLanguageSynchronizer hook to update the: 1. frontend-language to the user-preference - if there is one. 2. user-preference to the (browser-detected) frontend-language - otherwise.

🐛(frontend) allow left panel to update on language change

0e02db3

- fixes a bug where after language-sync the left panel would remain in the same language as before.

🚨(tests) add language related tests; fix getByRole not working in tests

2419c26

- adds tests and test-utility for solid language switching in tests - fixes where ...getByRole(menuitem... would not return a valid object

rvveber force-pushed the fix/email-in-receiving-user-language branch from b0a5b71 to 2419c26 Compare March 4, 2025 15:41

sampaccoud approved these changes Mar 5, 2025

View reviewed changes

sampaccoud enabled auto-merge (rebase) March 5, 2025 13:17

Merge branch 'main' into fix/email-in-receiving-user-language

1bb8330

sampaccoud merged commit 251787b into suitenumerique:main Mar 5, 2025
17 of 18 checks passed

AntoLC mentioned this pull request Mar 6, 2025

🔖(minor) release 2.4.0 #683

Merged


		import { getMatchingLocales } from './utils/locale';

		const LanguageInitializer = () => {

🐛(i18n/backend) Fix/email in receiving user language #401

🐛(i18n/backend) Fix/email in receiving user language #401

Uh oh!

Conversation

rvveber commented Nov 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rvveber commented Nov 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AntoLC left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AntoLC left a comment

Choose a reason for hiding this comment

Uh oh!

sampaccoud commented Dec 1, 2024

Uh oh!

Uh oh!

AntoLC left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rvveber Mar 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

virgile-dev commented Mar 5, 2025

Uh oh!

sampaccoud commented Mar 5, 2025

Uh oh!

Uh oh!

Uh oh!

rvveber commented Nov 4, 2024 •

edited

Loading

rvveber commented Nov 21, 2024 •

edited

Loading

rvveber Mar 3, 2025 •

edited

Loading